-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support pool reward tx #298
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's transfer to every address 1 ALPH from the testWallet in the integration test, so that we test the pool reward case there.
packages/web3/src/utils/exchange.ts
Outdated
for (const input of tx.unsigned.inputs) { | ||
try { | ||
const address = getAddressFromUnlockScript(input.unlockScript) | ||
inputAddresses.push(address) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There might be duplicated addresses in tx inputs
@@ -24,7 +24,7 @@ import { | |||
getSenderAddress, | |||
getALPHDepositInfo, | |||
isSimpleALPHTransferTx, | |||
isSimpleTransferTokenTx, | |||
isSimpleTokenTransferTx, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about removing these two isSimple*
functions? They are not recommended for exchanges now.
test/exchange.test.ts
Outdated
@@ -208,8 +208,8 @@ class Exchange { | |||
async handleBlock(block: node.BlockEntry, resolver: () => void) { | |||
for (const tx of block.transactions) { | |||
if (isSimpleALPHTransferTx(tx)) { | |||
const { targetAddress, depositAmount } = getALPHDepositInfo(tx) | |||
if (this.hotAddresses.includes(targetAddress)) { | |||
const infos = getALPHDepositInfo(tx).filter((v) => this.hotAddresses.includes(v.targetAddress)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should replace isSimpleALPHTransferTx
with the new validation function.
No description provided.